docs(adr): propose ADR-0010 and ADR-0011 — the two halves of offline browse - #53
Open
jeffcrouse wants to merge 4 commits into
Open
docs(adr): propose ADR-0010 and ADR-0011 — the two halves of offline browse#53jeffcrouse wants to merge 4 commits into
jeffcrouse wants to merge 4 commits into
Conversation
…pinned The engine already downloads every track whole before playing it and deletes the file when the next one loads. ADR-0009 recorded keeping those bytes as a follow-up rather than deciding it, because bytes retained because they happened to be fetched are a different promise from a track the listener asked for. This decides that follow-up, and can be rejected without disturbing ADR-0009. The mechanism is nearly free: `ManagedTempFile(url:owned:)` already means "do not delete this one", and what the engine lacks is a say in *where* the download lands. What is not free is the policy — a cache that is never evicted is a slow download of the entire library, while ADR-0009 point 10 promises nothing is evicted automatically. Both hold only if the store keeps two classes of file with two different promises, explicitly, with a test on the boundary. The web client's one recorded bug is exactly that boundary getting blurred: "metadata present, audio absent". Measured rather than assumed, and the measurement deliberately does not settle it. 63% of all bytes ever sent for playback were re-sends — 62 GB of 99 GB — but that is a 175-day aggregate with no ordering, and the only ordered data is two days of `play_events` drawn from the days the Swift client was being developed against this library. Simulated LRU over it gives 5.5% at 0.5 GB and saturates at 13.3%. The ADR claims no hit rate; a follow-up says to re-run the simulation on a month of real use and supersede this rather than implement it if the curve flattens too low. Line citations re-verified against `NativeAudioEngine.swift` today: the four `ManagedTempFile` sites and `cleanupTempFile` had all drifted 30-40 lines since drafting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW
…by delta ADR-0009 point 9 limited offline browse to the downloads list and deferred this, which leaves downloads as half a feature: with no network the Apple client can play what it downloaded but cannot navigate to it the way it was found. It also named this as the decision settling whether the `Codable` index becomes SQLite, so it wants deciding before that index acquires dependents. **The premise this was drafted from has since been fixed, and is kept as history rather than deleted.** `libraryCache.ts` passed `limit` to an endpoint that takes `page_size`, so "cache library for offline browsing" cached 50 tracks of 26,396 — and nothing caught it, because a 50-track offline library looks like a feature that works. That was fixed independently in #46 on 2026-07-30. The reasoning survives the fix: the ADR's argument was never "the web client has a bug" but that porting that design would port the shape of the defect, which is the same failure ADR-0012 point 3 rejects for favourites. What the fix removes is urgency, not the case. Re-measured against the live library today rather than trusting the draft: 26,396 active tracks, 132 requests at the `page_size` cap, and **13.2 MB** at 499 B/track — the draft said 12.5 MB, and its ten-field figure did not reconcile with its own per-track number (271 B × 26,396 is 7.1 MB, not 6.8). The `/library/stats` rejection holds exactly: 26,462 vs 26,396 tracks, 3,871 vs 3,925 albums, 3,662 vs 3,475 artists — three notions of "how many albums" and two of "how many tracks", none of them the set a client pages through. The three backend prerequisites are unchanged and still true: `TrackResponse` exposes no `updated_at`, `list_tracks` takes no `since`, and the scanner's upsert `set_` clause never mentions `updated_at` — so a rescan that rewrites a track's tags leaves the cursor untouched, which is a correctness bug on its own merits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW
… exists The follow-up explained at length that it was deliberately not linking ADR-0011 because the file was held on an unpushed branch and there was nothing to link to. There is now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW
The draft claimed a rescan that changes a track's tags leaves `updated_at` untouched, because the scanner's upsert omits it from an `on_conflict_do_update` `set_` clause SQLAlchemy emits verbatim. The omission was real. The conclusion was not: that upsert is in `_create_track`, the new-track path, whose conflict branch fires only when two scans race on the same `file_path`. An ordinary rescan goes through `_update_track`, which is plain ORM attribute assignment, so `onupdate` applies and the timestamp moves. Caught only by measuring it. The first two attempts reported a false positive, because `expire_on_commit=False` lets the in-memory attribute be older than the row it came from — reading back on a separate connection showed the timestamp advancing exactly when the rescan ran. This makes the ADR cheaper: two backend prerequisites rather than three. The race-path gap was fixed separately, with three tests pinning what a delta rests on — a retag moves the cursor, an unchanged rescan does not, and the conflict branch moves it. Recorded rather than quietly edited, per rule 5. A premise that made the work look *larger* is worth writing down for the same reason as one that made it look smaller: the next person to read `scanner.py` will see the same `set_` clause and reach the same wrong conclusion. Also corrected: the clause lists 22 columns, not 23. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW
This was referenced Jul 31, 2026
jeffcrouse
added a commit
that referenced
this pull request
Aug 1, 2026
…ay mark (#57) Every web play was landing with a completion ratio of almost exactly 0.5, whatever the listener actually heard. `usePlayTracking` delivered the play the moment listening crossed `min(duration / 2, 4 min)` and sent `completion_ratio` as measured *at that instant*. Nothing revised it afterwards, so the figure recorded when someone heard a track once through and when they heard half of it were the same number. Measured on the live database, 2026-08-01: **289 of 357 completed events sit in the 0.5–0.6 bucket**, while rows from the native client — which reports at the end — correctly read 0.95–1.00. Sample rows are exact: 166.0/331.4, 124.3/248.6, 120.6/241.0. This matters because of what the data is for. ADR-0004 exists so ADR-0005's recommender has a taste signal that can only accumulate in wall-clock time, and completion is that signal. A constant carries none of it, so the month of data `familiar` #53 is parked waiting for would have been a month of noise — and the error is invisible in aggregate, because 0.5 is a thoroughly plausible average. **Nothing changes about which plays count.** The early delivery only decided *when*: a play past 30s but short of the half mark was already reported on track change, by the same rule that now reports all of them. The Last.fm half/4-minute threshold turned out to gate nothing, which is why it is gone rather than moved. Two things that came with it: - **A page-hide flush**, because the early delivery was quietly buying durability: close the tab mid-track and the play still counted, since it had already been sent. Reporting at the end would have dropped that silently. Restored on `pagehide` and `visibilitychange`, with the ratio actually heard. Best effort and described as such — a browser tearing down a page owes the request nothing. - **The ratio is clamped.** Played time accumulates from the engine's clock while the duration is track metadata, so a looping or mis-tagged track could send 1.1. The server clamps as well, but agreeing with the native client — which has always clamped — beats leaving every stored figure a question about which end did it. Tests: the three that asserted mid-playback delivery now assert delivery on track change, and two were added — one pinning the ratio of a track heard nearly to the end, one covering the page-hide path and that it cannot double-report. 940 pass. Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 3, 2026
jeffcrouse
added a commit
that referenced
this pull request
Aug 3, 2026
…ruction (#80) Phase 1. Three decisions wait on ADR-0004's data — its own completion-ratio threshold, ADR-0005's RADIO weights, and #53 — and the data does not start where everyone assumed. Until #57 the web client delivered a play the moment listening crossed `min(duration / 2, 4 min)` and sent `completion_ratio` as measured at that instant, never revising it. Verified again on the live database on 2026-08-02, by day and context: **795 of 823 rows predate the fix**, and every context before 2026-08-01 shows completions averaging 0.42–0.50 — including `library`, because the web derives context from the queue source and sends `library` for a library queue exactly as the native app does. From 2026-08-01 the same context reads 0.972 and 1.000, with skips at ≤0.1. So the clock restarted on 2026-08-01, not 2026-07-27: a month of trustworthy data lands around 2026-09-01. `FEEDBACK_TRUSTWORTHY_SINCE` and `trustworthy_feedback_only` make the boundary one call away instead of a matter of discipline. A date, not a client filter, because `play_events` records no client and `context` does not stand in for one — the good native rows cannot be separated from the bad web rows beside them, and selecting on the ratio would be circular. Excluded rather than deleted, for a reason the investigation turned up: the damage is not limited to the ratio. A track abandoned at 55% was recorded as a completion at ~0.5 rather than as a skip, so `outcome` is unreliable before the cutoff and skips are *under*-counted. What survives is that `skipped` and `rejected` rows describe real abandonments. `ambient._negative_signal` counts exactly those two over a rolling 90-day window — so **the live recommender was never poisoned**, it has been running on a slightly weak negative signal that heals around 2026-10-30. ADR-0005 also gains the `Implementation:` block it never had, which is why it looked unbuilt in a survey while `RADIO` had been shipping for weeks. Two follow-ups recorded: a `client` column, cheap now and worthless retroactively, which is the only thing that would have made this separable; and that whether the web still reports at all is unverified — every post-cutoff row carries the context the native app hardcodes. Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
jeffcrouse
added a commit
that referenced
this pull request
Aug 3, 2026
CLAUDE.md loads into context every session, so a stale "what remains" is not a tidy-up — it is a wrong instruction delivered on every turn. Three of them: - ADR-0017's paragraph said the `WKWebView`, the message handler and the native unavailable state were still to build. They shipped in `familiar-apple` #43, and #53 brought the surface to the phone. - ADR-0018's said "`0018` ships without Discover if `0019` is not accepted". Both are accepted and shipped. - ADR-0022's said accepting it makes #76 reversible via a third bridge message. It did not go that way: `/library/discover/prompts` was already generated, so Listening Ideas came back natively and **the bridge stays at two messages**. That sentence would have steered the next session into building the thing we deliberately did not build, which is the worst kind of stale. ADR-0017's own Implementation block carried the same claim — an accepted ADR is a living record, and rule 6 protects the Decision, not the record of what shipped. It now also lists the three defects that surface has produced, all one shape: an affordance whose destination is not mounted, failing silently. ADR-0022 gains its Implementation block: what shipped, the three places the server did not match its documentation, and the empty-line SSE bug the ADR's own recorded tradeoff predicted. Also: the Music Map ordering note is spent, so it now records that the map's interaction is still half-wired — the footer advertises scroll-to-zoom that was never implemented, `zoomed(by:toward:)` is reachable only through `stepped()`, and the drag gesture competes with click-to-focus. Verified against the source, not from memory. Key Files gains rows for the web chat and the embedded surface, neither of which had a pointer. Every PR number, path and claim re-checked (rule 4). `useChatAvailability.ts` was dropped from Key Files because it exists only in the open #78, and the two open PRs are marked as open rather than described as shipped. Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two decisions ADR-0009 explicitly deferred, proposed together because they are a pair: 0010 decides
what happens to bytes the engine already fetched, 0011 decides whether the library can be browsed
without a server. ADR-0012's offline follow-up is blocked on 0011, which is what brought them up
now.
They are separately approvable. 0010 can be rejected without disturbing anything; 0011 has real
prerequisites.
ADR-0010 — played bytes are cached, downloads are pinned
The engine already downloads every track whole before playing it and deletes it when the next one
loads. Keeping those bytes is nearly free —
ManagedTempFile(url:owned:)already means "do notdelete this one"; what the engine lacks is a say in where the download lands.
The policy is not free, and that is why it is a decision rather than a patch. A cache that is never
evicted is a slow download of the entire library, while ADR-0009 point 10 promises nothing is
evicted automatically. Both hold only if the store keeps two classes of file with two different
promises, explicitly, with a test on the boundary — which is precisely the boundary the web client
already got wrong once ("metadata present, audio absent").
The measurement deliberately does not settle it. 63% of all bytes ever sent for playback were
re-sends (62 GB of 99 GB) — but that is a 175-day aggregate with no ordering, and the only ordered
data is two days of
play_eventsdrawn from the days the Swift client was being developedagainst this library. Simulated LRU gives 5.5% at 0.5 GB, saturating at 13.3%. The ADR claims no hit
rate, and a follow-up says to re-run the simulation on a month of real use and supersede rather
than implement if the curve flattens too low.
ADR-0011 — the library is cached whole, refreshed by delta
Caches all 26,396 tracks (13.2 MB, 132 requests), keeps albums and artists as the server returns them
rather than deriving them, stores files rather than SQLite — which is the answer ADR-0009 deferred —
and refreshes by delta behind a purpose-built fingerprint endpoint.
One premise died and is kept as history rather than deleted.
libraryCache.tspassedlimittoan endpoint that takes
page_size, so "cache library for offline browsing" cached 50 tracks of26,396. That was fixed independently in #46 the day after this was drafted. The reasoning survives:
the argument was never "the web client has a bug" but that porting that design would port the shape
of the defect — a cache that silently holds part of a collection is indistinguishable from a working
one, which is the same failure ADR-0012 point 3 rejects for favourites.
Verification before proposing
Re-measured against the live library and re-checked against current code today, not trusted from the
draft:
page_sizecap, 13.2 MB at 499 B/track — the draftsaid 12.5 MB, and its ten-field figure did not reconcile with its own per-track number
(271 B × 26,396 is 7.1 MB, not 6.8).
/library/statsrejection holds exactly: 26,462 vs 26,396 tracks, 3,871 vs 3,925 albums,3,662 vs 3,475 artists.
updated_atonTrackResponse, nosinceonlist_tracks, and noupdated_atanywhere in the scanner — so a rescan that rewrites a track'stags leaves the cursor untouched. That last one is a correctness bug worth fixing on its own
merits, whatever happens to this ADR.
🤖 Generated with Claude Code
https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW